Repository navigation
Conversation
ApacheDS modifies incoming search filters by ORing with (objectClass=referral). The determination by the LDAPRolesLookupInterceptor for whether a search is an "all groups" did not handle this case properly to ensure that broader searches would still pass through to the LDAP backends. This commit specifically recognizes (objectClass=referral) when determining if a search filter is for "all groups". Unit tests were modified to cover this filter. The workload test was unskipped.
| return ((OrNode) filter).getChildren().stream().allMatch(this::isGroupObjectClassFilter); | ||
| // Remove any (objectClass=referral) nodes. An OR is an all groups filter only if every child is. | ||
| return ((OrNode) filter).getChildren().stream() | ||
| .filter(node -> !isReferralObjectClassFilter(node)) |
There was a problem hiding this comment.
ApacheDS's SearchRequestHandler.modifyFilter doesn't flatten the filter. It builds new OrNode(clientFilter, (objectClass=referral)). Removing the referral child here only helps when the client filter is a single objectClass equality, because the remaining child still has to pass isGroupObjectClassFilter. Once wrapped, these root searches are still not detected as role listings (I checked with a unit test that wraps the filter the same way):
(&(objectClass=groupOfNames)(cn=*))(&(objectClass=groupOfNames)(|(cn=viewer)(cn=*)))(|(objectClass=groupOfUniqueNames)(objectClass=groupOfNames))
Could we follow the JIRA instead and unwrap once in isAllGroupsSearch (line 118)? If the filter is a 2-child OrNode and one child is (objectClass=referral), use the other child as searchFilter. Then the existing isAllGroupsFilter/isAllGroupsFilterForAncestor logic works unchanged, and this OR branch can go back to how it was.
| "(&(objectClass=groupOfNames)(|(cn=viewer)(cn=*)))", | ||
| "(|(objectClass=groupOfUniqueNames)(objectClass=groupOfNames))")) { | ||
| "(|(objectClass=groupOfUniqueNames)(objectClass=groupOfNames))", | ||
| "(|(objectClass=groupOfNames)(objectClass=referral))", |
There was a problem hiding this comment.
These two cases hand-write the wrapped form for the simple filters only. The other entries in this list (the AND and nested-OR ones) still go to the interceptor unwrapped, which never happens in production unless the client sends ManageDsaIT. That's the same blind spot the JIRA describes. Could we add a helper that wraps the way ApacheDS does, e.g. new OrNode(FilterParser.parse(schemaManager, filter), FilterParser.parse(schemaManager, "(objectClass=referral)")), and use it for every filter in this test? The JIRA also asks for the negative side: please wrap the filters in testRootSearchWithNonGroupFilterIsNotAllGroupsSearch (line 688) as well, so non-group filters are still not treated as listings once wrapped.
Test Results 5 files 5 suites 1m 25s ⏱️ Results for commit 8126bc1. |
updated unit tests with more complicated test cases and changed all groups detection to remove the referral if the top level filter is a 2-child or-node.
KNOX-3546 - Fix all groups search in roles lookup interceptor
What changes were proposed in this pull request?
ApacheDS modifies incoming search filters by ORing with (objectClass=referral). The determination by the LDAPRolesLookupInterceptor for whether a search is an "all groups" did not handle this case properly to ensure that broader searches would still pass through to the LDAP backends.
This commit specifically recognizes (objectClass=referral) when determining if a search filter is for "all groups".
How was this patch tested?
Unit tests were modified to cover this filter.
Integration Tests
The workload test was unskipped and run
UI changes
no UI changes